quality of life fix for pip users#1289
Conversation
|
Thanks for the PR! I think it's a great idea to allow this selection of packages. I have not reviewed the code yet so just two quick comments:
Once #1290 is merged I'll come back to this PR. |
carlosggarcia
left a comment
carlosggarcia
left a comment
There was a problem hiding this comment.
Thanks for the work! Just a few requests.
|
|
||
| with mock.patch.dict(sys.modules, {'isitgr': None}): | ||
| with pytest.raises(ModuleNotFoundError): | ||
| with pytest.raises(ccl.CCLError): |
There was a problem hiding this comment.
Why this change? Isn't ModuleNotFoundError a more explicit error?
| return False | ||
|
|
||
|
|
||
| _DEFAULT_TRANSFER_FUNCTION = 'boltzmann_camb' if _camb_available() else 'eisenstein_hu' |
There was a problem hiding this comment.
We have to keep 'boltzmann_camb' as default to avoid changing the API. Also, I don't think it's a good idea to have this kind of automatic changes depending on what's installed because one can end up with the wrong transfer function inadvertently.
| T_CMB=DefaultParams.T_CMB, | ||
| T_ncdm=DefaultParams.T_ncdm, | ||
| transfer_function='boltzmann_camb', | ||
| transfer_function=_DEFAULT_TRANSFER_FUNCTION, |
There was a problem hiding this comment.
As said above. Revert this.
| pip install pyccl[full] # All of the above | ||
| ``` | ||
|
|
||
| Without `pyccl[boltzmann]`, the default `transfer_function` is `'eisenstein_hu'`. |
There was a problem hiding this comment.
As I said above, we won't change the default transfer function so remove these lines.
| "pytest-cov", | ||
| ] | ||
|
|
||
| boltzmann = ["camb", "classy", "isitgr"] |
There was a problem hiding this comment.
i would keep isitgr out and create it its own optional set e.g. MG, since most people won't need it.
2 participants
This pull request adds quality-of-life support for pip users by adding optional dependency groups in
pyccl, making it easier for users to install only the packages they need for specific functionalities (e.g., Boltzmann codes, perturbation theory, emulators). It updates the documentation to explain these new installation options, improves error handling for missing optional dependencies, and sets the default transfer function based on installed packages.Existing Issue this Fixes: when you install the base package via pip, Boltzmann codes are not installed but the default
transfer_functioninccl.Cosmology()is'boltzmann_camb'. This creates a problem when you go to use the cosmology object in components of the halo model for example. Now with these changes, I've made the default behavior be that if the Boltzmann codes are not installed,'eisenstein_hu'is the default, otherwise camb is the default (I believe the default behavior in the conda installation).Dependency management and installation:
boltzmann,pt,emulators,full) topyproject.tomlfor easier installation of extra features.README.mdandreadthedocs/source/installation.rstto document the new extras and clarify default behaviors depending on installed packages. [1] [2] [3] [4] [5]Default behavior and configuration:
transfer_functionis now set based on whethercambis installed:'boltzmann_camb'if available, otherwise'eisenstein_hu'. [1] [2]Error handling improvements:
pyccl/boltzmann.pyto raise aCCLErrorwith clear installation instructions if a required optional dependency (CAMB, CLASS/classy, ISiTGR) is missing, instead of a genericImportError. [1] [2] [3]CCLErrorwhen optional dependencies are missing.